Conversation
…update # Conflicts: # apps/webapp/app/components/logs/LogsTaskFilter.tsx # apps/webapp/app/components/runs/v3/RunFilters.tsx
|
WalkthroughThe PR introduces wide UI and filtering changes: adds keyboard shortcuts and tooltip anchors to many filter controls; standardizes spacing and text/icon color classes on Select triggers and items; converts multiple on-demand dropdown filters into permanent controls; extracts and exports a reusable IdFilterDropdown and adds ScopeFilterProps/controlled ScopeFilter mode; extends primitives and hooks with new props/behavior (Select: Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
| const { filteredItems } = useFuzzyFilter<TaskListItem>({ | ||
| items: tasks, | ||
| keys: ["slug", "filePath", "triggerSource"], | ||
| filterText: value("search") ?? "", | ||
| }); |
There was a problem hiding this comment.
🚩 Tasks page search changed from instant client-side to Enter-to-search with server round-trips
The tasks page (_index/route.tsx:178-182) replaced the local-only AnimatedSearchField (which filtered on every keystroke via useFuzzyFilter's internal state) with SearchInput (which writes to URL params only on Enter). The useFuzzyFilter hook now reads from value("search") which only updates after URL navigation. This has two effects: (1) filtering requires pressing Enter instead of being instant as-you-type, and (2) the URL param change triggers the Remix loader to re-run, re-fetching all task data from the server even though the loader at _index/route.tsx:130-145 does not use the search param. The same pattern applies to the environment variables page at environment-variables/route.tsx:268-277. This appears to be an intentional choice for UI consistency, but reviewers should confirm the UX tradeoff is acceptable, especially the unnecessary server round-trips.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
The loader doesn't filter by search, and useFuzzyFilter runs client-side on the already-loaded tasks, so the URL update re-renders the filtered list instantly — the user doesn't wait on the round-trip. Remix revalidates in the background because the URL changed, but the returned data is identical so there's no UX impact. Net cost is one extra taskListPresenter query per Enter press, async and off the critical path. We can suppress it with a shouldRevalidate that ignores search-only changes if we want to be tidy, but it's not a correctness or perceivable-perf concern.
There was a problem hiding this comment.
The loader doesn't filter by search, and useFuzzyFilter runs client-side on the already-loaded tasks, so the URL update re-renders the filtered list instantly — the user doesn't wait on the round-trip. Remix revalidates in the background because the URL changed, but the returned data is identical so there's no UX impact. Net cost is one extra taskListPresenter query per Enter press, async and off the critical path. We can suppress it with a shouldRevalidate that ignores search-only changes if we want to be tidy, but it's not a correctness or perceivable-perf concern.
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/webapp/app/components/primitives/Select.tsx (1)
358-374:⚠️ Potential issue | 🟠 MajorTooltip is disabled when only
tooltipTitleis provided (no shortcut).
showTooltipbecomes truthy fortooltipTitle || shortcut, so theAriakit.Tooltiprenders, butdisabled={shortcut === undefined}keeps it suppressed whenever there's no shortcut. Since many call sites passshortcut={shortcut}(where shortcut is a variable that can be undefined), this will break tooltip display. Updatedisabledto enable the tooltip when eithertooltipTitleorshortcutis present.🛠️ Suggested fix
{showTooltip && ( <Ariakit.Tooltip - disabled={shortcut === undefined} + disabled={!tooltipTitle && shortcut === undefined} className="z-40 cursor-default rounded border border-charcoal-700 bg-background-bright px-2 py-1.5 text-xs" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/primitives/Select.tsx` around lines 358 - 374, The Tooltip is being rendered when showTooltip (tooltipTitle || shortcut) is truthy but remains disabled whenever shortcut is undefined; update the disabled prop on Ariakit.Tooltip so it is enabled when either tooltipTitle or shortcut is present (e.g., make disabled true only when both tooltipTitle and shortcut are absent). Locate the Ariakit.Tooltip usage and change disabled={shortcut === undefined} to a condition that reflects both tooltipTitle and shortcut (referencing the tooltipTitle, shortcut and showTooltip variables and the ShortcutKey usage to verify behavior).apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx (1)
65-89:⚠️ Potential issue | 🟡 MinorKeep the clear-all visibility check aligned with the active filters.
This local
hasFiltersonly looks atstatuses,tags,id, andidempotencyKey, so a time-only state (period/from/to) won't show the clear button even though submitting this form clears that time filter too. Reuse the incominghasFiltersprop here, or include the time params in the local check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx` around lines 65 - 89, The local hasFilters in WaitpointTokenFilters only checks "statuses", "tags", "id", and "idempotencyKey", so the clear-all button is hidden when only time filters are active; update the component to either use the incoming props.hasFilters instead of the local searchParams check, or extend the local check to include "period", "from", and "to" (i.e., inspect location.search for those keys). Modify the hasFilters reference used in the JSX (the conditional rendering that shows the Form and Button) to use the corrected boolean so time-only filters make the clear button visible; refer to the WaitpointTokenFilters function and the hasFilters variable to locate the change.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsx (1)
447-489:⚠️ Potential issue | 🟠 MajorRe-use the shared create controls in the empty state.
When
totalWidgetCount === 0, this branch renders only a plain “Add chart” button and never mountsfilterAccessories. That drops the new “Add title” action entirely and also skips the shortcut/tooltip wiring you added to the shared controls, so empty dashboards now behave differently from populated ones.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsx around lines 447 - 489, The empty-state branch (totalWidgetCount === 0) currently renders a lone Add chart Button and never mounts the shared controls (filterAccessories), which removes the "Add title" action and shortcut/tooltip wiring; fix this by reusing the same shared control set used by MetricDashboard: render filterAccessories (or the shared DashboardControls component) inside the empty-state InfoPanel accessory (or directly render the shared accessory node alongside the Add chart button) so that actions.openAddEditor, filterAccessories, and the shortcut/tooltip wiring are mounted for the empty dashboard just like for MetricDashboard.
🧹 Nitpick comments (8)
apps/webapp/app/components/metrics/ModelsFilter.tsx (1)
155-155: Droptext-text-brighton the disabled empty-state item.The other filter dropdowns in this PR (
QueuesFilter,ProvidersFilter,OperationsFilter) leave the "No X found"SelectItemwithouttext-text-bright, so the disabled placeholder reads as muted helper text. Applyingtext-text-brighthere makes ModelsFilter's empty state visually inconsistent.♻️ Proposed fix
- {filtered.length === 0 && <SelectItem disabled className="text-text-bright">No models found</SelectItem>} + {filtered.length === 0 && <SelectItem disabled>No models found</SelectItem>}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/metrics/ModelsFilter.tsx` at line 155, In ModelsFilter, remove the explicit "text-text-bright" class from the disabled empty-state SelectItem so it matches the other filters; locate the JSX that renders "{filtered.length === 0 && <SelectItem disabled className="text-text-bright">No models found</SelectItem>}" and change it to render the disabled SelectItem without the text-text-bright class (keep the disabled prop and inner text "No models found" intact).apps/webapp/app/components/runs/v3/SharedFilters.tsx (1)
1035-1044: Drop the unnecessaryuseCallbackand a stale-closure dep.
applyis just an inline click handler used once at the JSX site (line 1090) — no dep array, no provider, no expensive derivation. Wrapping it inuseCallbackadds no benefit and the dep list is also missingclearSearchValue(the captured closure may become stale if the parent re-creates it).♻️ Suggested simplification
- const apply = useCallback(() => { + const apply = () => { clearSearchValue(); replace({ cursor: undefined, direction: undefined, [paramKey]: inputValue === "" ? undefined : inputValue?.toString(), }); setOpen(false); - }, [inputValue, replace, paramKey]); + };As per coding guidelines: "Only use
useCallback/useMemofor context provider values, expensive derived data that is a dependency elsewhere, or stable refs required by a dependency array. Don't wrap ordinary event handlers or trivial computations".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/runs/v3/SharedFilters.tsx` around lines 1035 - 1044, The apply function is unnecessarily wrapped in useCallback and captures a stale closure (missing clearSearchValue in deps); remove the useCallback wrapper and replace it with a plain inline function (or const apply = () => { ... } defined without useCallback) used at the JSX click site, ensuring it still calls clearSearchValue(), replace({ cursor: undefined, direction: undefined, [paramKey]: inputValue === "" ? undefined : inputValue?.toString() }), and setOpen(false); this eliminates the incorrect dependency array and stale-closure risk around clearSearchValue, inputValue, replace, paramKey, and setOpen.apps/webapp/app/hooks/useFuzzyFilter.ts (1)
36-61: Minor: clarify thatsetFilterTextis uncontrolled-only.In controlled mode (
filterTextprop is provided), the returnedsetFilterTextonly mutatesinternalFilterText, which is then shadowed by the controlled value and effectively a no-op. Consumers in controlled mode that wire this setter to anonChangewill see no updates unless they also propagate to the source offilterText. The current top-level JSDoc still describessetFilterTextas "Function to update the filter text", which could mislead future callers.📝 Suggested doc tweak
* `@returns` An object containing: * - filterText: The current filter text - * - setFilterText: Function to update the filter text + * - setFilterText: Updates the internal filter text. Ignored when the controlled + * `filterText` prop is provided — manage that state in the parent instead. * - filteredItems: The filtered array of items based on the current filter text🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/hooks/useFuzzyFilter.ts` around lines 36 - 61, The returned setter setFilterText currently only updates internalFilterText and is a no-op when controlledFilterText is provided; update useFuzzyFilter's API docs and behavior to avoid misleading callers: in the JSDoc for useFuzzyFilter (and the exported return shape) explicitly state that setFilterText only updates internal state when uncontrolled (i.e., when controlledFilterText is undefined) and will not override a provided controlledFilterText, and optionally replace the raw setInternalFilterText return with a conditional setter that either calls setInternalFilterText or, when controlledFilterText is present, logs a clear warning/returns a no-op; reference the symbols filterText, setFilterText, internalFilterText, controlledFilterText, and function useFuzzyFilter so reviewers can locate and apply the change.apps/webapp/app/components/runs/v3/ScheduleFilters.tsx (1)
75-85: Extract the"ALL"select value into a shared constant.This sentinel is duplicated across both permanent filters in comparisons, defaults, and option values. A single constant will keep the UI and URL handling from drifting the next time one of these filters changes.
As per coding guidelines, use named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons.Also applies to: 97-127, 166-176, 186-217
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/runs/v3/ScheduleFilters.tsx` around lines 75 - 85, Define a single named constant for the sentinel select value (e.g., export const UNSET_VALUE = '__unset__') and replace all raw "ALL" string literals in ScheduleFilters (including inside handleChange, other change handlers, defaults, and option value attributes) with this constant so comparisons (selected === UNSET_VALUE), default values, and option value props stay in sync; update URL handling logic that sets/deletes the "type" param to treat UNSET_VALUE as meaning unset and ensure any tests or consumers reference the new constant as well.apps/webapp/app/components/runs/v3/RunFilters.tsx (2)
589-657: LGTM on the permanent status/task filter pattern.The
Ariakit.TooltipProvider+TooltipAnchor(render={<Ariakit.Select … />})+ programmatictriggerRef.current?.click()shortcut wiring is consistent acrossPermanentStatusFilter,PermanentTasksFilter,RootOnlyToggle, and the newScopeFilter— good consolidation. One small nit: theuseShortcutKeys({ shortcut: statusShortcut, ... })calls here don’t passdisabled, whileScopeFilterpassesdisabled: !shortcut; sincestatusShortcut/tasksShortcutare always-defined module-level constants, that’s fine, but consider extracting a tinyuseTriggerShortcut(triggerRef, shortcut)helper to remove the duplicated 7-line block in three places.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/runs/v3/RunFilters.tsx` around lines 589 - 657, The shortcut wiring in PermanentStatusFilter (and the similar blocks in PermanentTasksFilter, RootOnlyToggle, and ScopeFilter) is duplicated; extract a small helper useTriggerShortcut(triggerRef, shortcut) that calls useShortcutKeys with the same action (preventDefault/stopPropagation then triggerRef.current?.click()) and passes disabled: !shortcut, then replace the 7-line inline useShortcutKeys blocks in PermanentStatusFilter, PermanentTasksFilter, RootOnlyToggle, and ScopeFilter with calls to useTriggerShortcut using their trigger refs and shortcut constants to remove duplication.
59-61: Minor: import grouping.
ShortcutKey(a primitive component) is wedged between two hook imports (useShortcutKeysandtagsLoader). Move it up next to the other~/components/primitives/*imports for consistency with the rest of the file. (Skip if Prettier/import-sort isn’t enforced here.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/runs/v3/RunFilters.tsx` around lines 59 - 61, The import for the primitive component ShortcutKey is currently between two hook imports; move the ShortcutKey import so it sits with the other primitive/component imports (e.g., alongside other imports from ~/components/primitives/*) instead of between useShortcutKeys and tagsLoader—adjust the import order in RunFilters.tsx so useShortcutKeys and tagsLoader remain together and ShortcutKey is grouped with components.apps/webapp/app/components/metrics/ScopeFilter.tsx (2)
56-82: Minor: missingeslint-disable-next-lineon theas anycast.Line 62 uses
ref={triggerRef as any}but, unlike the equivalent pattern inRunFilters.tsx(e.g. lines 614-616 and 784-786), there is no// eslint-disable-next-line@typescript-eslint/no-explicit-any`` comment. If the repo's ESLint rules forbid explicitany, this will emit a lint error. Consider adding the same disable comment for consistency, or typing the ref as `HTMLDivElement` (since `Ariakit.Select` renders a `` here and `HTMLDivElement.click()` exists on `HTMLElement`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/metrics/ScopeFilter.tsx` around lines 56 - 82, The ref passed to Ariakit.Select uses an explicit any cast (ref={triggerRef as any}) which triggers the no-explicit-any lint rule; fix by either adding the same eslint disable comment used in RunFilters (// eslint-disable-next-line `@typescript-eslint/no-explicit-any`) directly above the ref usage, or better, change the triggerRef typing to a proper element type (e.g., HTMLDivElement | null) and remove the as any cast so Ariakit.Select receives a correctly typed ref; locate the usage of triggerRef and the Ariakit.Select render prop to apply the change.
22-44: Optional: enforceonValueChangeat the type level when controlled.The JSDoc says
onValueChangeis "Required whenvalueis provided", but the type allowsvaluewithoutonValueChange, andhandleChangethen silently no-ops viaonValueChange?.(...). Consider a discriminated union so misuse is caught at compile time:♻️ Proposed refactor
-export type ScopeFilterProps = { - shortcut?: ShortcutDefinition; - /** Controlled value. If provided, the filter uses controlled mode and ignores search params. */ - value?: QueryScope; - /** Called when the user selects a new scope. Required when `value` is provided. */ - onValueChange?: (scope: QueryScope) => void; -}; +export type ScopeFilterProps = + | { + shortcut?: ShortcutDefinition; + value?: undefined; + onValueChange?: undefined; + } + | { + shortcut?: ShortcutDefinition; + /** Controlled value; the filter ignores search params in this mode. */ + value: QueryScope; + /** Called when the user selects a new scope. */ + onValueChange: (scope: QueryScope) => void; + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/metrics/ScopeFilter.tsx` around lines 22 - 44, ScopeFilterProps should be a discriminated union so providing value requires onValueChange at the type level; replace the current interface with a union like Controlled (has value: QueryScope and onValueChange: (scope: QueryScope) => void) | Uncontrolled (no value and optional/absent onValueChange) and update the ScopeFilter signature to accept that union (keep shortcut optional), then simplify handleChange to call onValueChange(newScope) without optional chaining when in the controlled branch and only call replace(...) in the uncontrolled branch; refer to the ScopeFilterProps type, the ScopeFilter function parameters, and the handleChange implementation when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/webapp/app/components/metrics/ModelsFilter.tsx`:
- Line 19: The disabled "No models found" SelectItem in the ModelsFilter
component is using the bright utility class `text-text-bright`; remove that
class so the empty-state disabled item is muted like in QueuesFilter and
ProvidersFilter. Locate the SelectItem rendering for the "No models found"
option inside the ModelsFilter component and delete the `text-text-bright` class
(or replace it with the same muted class used by the other filters) to restore
visual consistency.
In `@apps/webapp/app/components/navigation/DashboardDialogs.tsx`:
- Around line 6-7: Remove the unused imports useRef and useShortcutKeys from the
top of DashboardDialogs.tsx and keep only the used symbols; replace the current
import so it imports useEffect and useState plus the ShortcutDefinition type
(preferably as a type-only import: import type { ShortcutDefinition } ...) since
ShortcutDefinition is only used as a type for the shortcut parameter in the
function referenced by ShortcutDefinition; ensure no other references to useRef
or useShortcutKeys remain in the file.
In `@apps/webapp/app/components/primitives/SearchInput.tsx`:
- Around line 100-110: The clear button currently uses onPointerDown with
preventDefault which preserves input focus for mouse but breaks keyboard
activation; change the button to use onMouseDown={(e) => e.preventDefault()} to
keep focus on mouse and add an onClick={() => handleClear()} so keyboard
Enter/Space triggers the action; update the button that renders <XMarkIcon />
and the handler reference handleClear accordingly, and apply the same
onMouseDown + onClick pattern to the analogous control in AIFilterInput (the
same clear button logic) to restore keyboard accessibility.
In `@apps/webapp/app/components/runs/v3/BatchFilters.tsx`:
- Around line 230-233: The validateBatchId function currently checks for lengths
27 or 31 but returns the incorrect message "27/32 characters long"; update the
error string in validateBatchId to read "27 or 31 characters long" so it matches
the allowed lengths and aligns with the similar RunFilters.tsx validator; ensure
the two conditions remain the same (startsWith "batch_" and length check) and
only the returned message is changed.
- Around line 184-187: The Ariakit.Select usage in BatchFilters.tsx is casting
triggerRef to any (ref={triggerRef as any}) which bypasses type safety; remove
the unnecessary casts and any related eslint-disable comments and use
ref={triggerRef} directly since triggerRef is a useRef<HTMLButtonElement> (same
pattern as TimeFilter in SharedFilters.tsx). Apply this change to both the
Status filter and the batch-id filter Ariakit.Select instances (and consider
doing the same cleanup in RunFilters.tsx for its triggerRef usages). Ensure
imports/types remain unchanged and the components compile without the casts.
In `@apps/webapp/app/components/runs/v3/RunFilters.tsx`:
- Around line 1640-1643: The validator validateScheduleId currently uses
startsWith("sched") which permits IDs without the required underscore; update
the check to startsWith("sched_") so it enforces the placeholder/error text,
keeping the existing length check and return messages (i.e., ensure
validateScheduleId uses startsWith("sched_") and still returns "Schedule IDs
start with 'sched_'" on failure).
- Around line 1497-1532: The RootOnlyToggle currently only updates the rootOnly
param and must also clear pagination so old cursors don't point past the
filtered results: in RootOnlyToggle (useSearchParams, replace) update the
replace call to remove/reset the "cursor" and "direction" params whenever
rootOnly changes (i.e., when onCheckedChange runs), matching the behavior of
other handlers like TasksDropdown.handleChange and StatusDropdown.handleChange
by deleting or omitting "cursor" and "direction" from the new search params you
pass to replace.
- Around line 1534-1537: The validateRunId function currently checks for lengths
25 or 29 but returns an error message saying "25/30", causing inconsistency;
update the user-facing error text in validateRunId to match the actual allowed
lengths (e.g., return "Run IDs are 25/29 characters long") so the message
matches the check (function name: validateRunId).
In `@apps/webapp/app/components/runs/v3/SharedFilters.tsx`:
- Around line 1029-1075: The inputValue state is initialized from currentValue
but never synced when currentValue changes externally, causing stale values in
the Input inside the SelectPopover; update the component to synchronize
inputValue with currentValue by adding an effect (watching currentValue) that
calls setInputValue(currentValue) and/or reset inputValue when the popover opens
(watch open) so Input shows the latest URL/search param state; reference the
inputValue/currentValue state variables, setInputValue setter, open/setOpen from
SelectProvider, and the SelectPopover/IdFilterDropdown wrapping this input to
locate where to add the useEffect.
In `@apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx`:
- Around line 303-313: The ComboBox is being unmounted when filtered.length ===
0 which hides the input; update the rendering condition around the ComboBox (the
JSX that currently uses !(filtered.length === 0 && fetcher.state !== "loading"))
to also keep the component mounted whenever searchValue is non-empty (e.g., only
hide when filtered is empty, fetcher is not loading, AND searchValue is empty).
Locate the ComboBox block in WaitpointTokenFilters.tsx and change the condition
so the input remains rendered while searchValue has content.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardKey/route.tsx:
- Around line 248-254: hasAppliedFilters currently only checks
tasks/queues/models/prompts/operations/providers and ignores time and scope
changes; update the logic in MetricDashboard where hasAppliedFilters is defined
(the const hasAppliedFilters) to also include checks for scope, period, from,
and to (e.g., scope !== defaultScope OR period !== defaultPeriod OR from/to
timestamps present or different from defaults) so that changing dashboard scope
or time range counts as an applied filter; make the same change to the other
occurrence of hasAppliedFilters in this file to ensure the “Clear all filters”
button appears when only scope/time range changes.
---
Outside diff comments:
In `@apps/webapp/app/components/primitives/Select.tsx`:
- Around line 358-374: The Tooltip is being rendered when showTooltip
(tooltipTitle || shortcut) is truthy but remains disabled whenever shortcut is
undefined; update the disabled prop on Ariakit.Tooltip so it is enabled when
either tooltipTitle or shortcut is present (e.g., make disabled true only when
both tooltipTitle and shortcut are absent). Locate the Ariakit.Tooltip usage and
change disabled={shortcut === undefined} to a condition that reflects both
tooltipTitle and shortcut (referencing the tooltipTitle, shortcut and
showTooltip variables and the ShortcutKey usage to verify behavior).
In `@apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx`:
- Around line 65-89: The local hasFilters in WaitpointTokenFilters only checks
"statuses", "tags", "id", and "idempotencyKey", so the clear-all button is
hidden when only time filters are active; update the component to either use the
incoming props.hasFilters instead of the local searchParams check, or extend the
local check to include "period", "from", and "to" (i.e., inspect location.search
for those keys). Modify the hasFilters reference used in the JSX (the
conditional rendering that shows the Form and Button) to use the corrected
boolean so time-only filters make the clear button visible; refer to the
WaitpointTokenFilters function and the hasFilters variable to locate the change.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsx:
- Around line 447-489: The empty-state branch (totalWidgetCount === 0) currently
renders a lone Add chart Button and never mounts the shared controls
(filterAccessories), which removes the "Add title" action and shortcut/tooltip
wiring; fix this by reusing the same shared control set used by MetricDashboard:
render filterAccessories (or the shared DashboardControls component) inside the
empty-state InfoPanel accessory (or directly render the shared accessory node
alongside the Add chart button) so that actions.openAddEditor,
filterAccessories, and the shortcut/tooltip wiring are mounted for the empty
dashboard just like for MetricDashboard.
---
Nitpick comments:
In `@apps/webapp/app/components/metrics/ModelsFilter.tsx`:
- Line 155: In ModelsFilter, remove the explicit "text-text-bright" class from
the disabled empty-state SelectItem so it matches the other filters; locate the
JSX that renders "{filtered.length === 0 && <SelectItem disabled
className="text-text-bright">No models found</SelectItem>}" and change it to
render the disabled SelectItem without the text-text-bright class (keep the
disabled prop and inner text "No models found" intact).
In `@apps/webapp/app/components/metrics/ScopeFilter.tsx`:
- Around line 56-82: The ref passed to Ariakit.Select uses an explicit any cast
(ref={triggerRef as any}) which triggers the no-explicit-any lint rule; fix by
either adding the same eslint disable comment used in RunFilters (//
eslint-disable-next-line `@typescript-eslint/no-explicit-any`) directly above the
ref usage, or better, change the triggerRef typing to a proper element type
(e.g., HTMLDivElement | null) and remove the as any cast so Ariakit.Select
receives a correctly typed ref; locate the usage of triggerRef and the
Ariakit.Select render prop to apply the change.
- Around line 22-44: ScopeFilterProps should be a discriminated union so
providing value requires onValueChange at the type level; replace the current
interface with a union like Controlled (has value: QueryScope and onValueChange:
(scope: QueryScope) => void) | Uncontrolled (no value and optional/absent
onValueChange) and update the ScopeFilter signature to accept that union (keep
shortcut optional), then simplify handleChange to call onValueChange(newScope)
without optional chaining when in the controlled branch and only call
replace(...) in the uncontrolled branch; refer to the ScopeFilterProps type, the
ScopeFilter function parameters, and the handleChange implementation when making
the change.
In `@apps/webapp/app/components/runs/v3/RunFilters.tsx`:
- Around line 589-657: The shortcut wiring in PermanentStatusFilter (and the
similar blocks in PermanentTasksFilter, RootOnlyToggle, and ScopeFilter) is
duplicated; extract a small helper useTriggerShortcut(triggerRef, shortcut) that
calls useShortcutKeys with the same action (preventDefault/stopPropagation then
triggerRef.current?.click()) and passes disabled: !shortcut, then replace the
7-line inline useShortcutKeys blocks in PermanentStatusFilter,
PermanentTasksFilter, RootOnlyToggle, and ScopeFilter with calls to
useTriggerShortcut using their trigger refs and shortcut constants to remove
duplication.
- Around line 59-61: The import for the primitive component ShortcutKey is
currently between two hook imports; move the ShortcutKey import so it sits with
the other primitive/component imports (e.g., alongside other imports from
~/components/primitives/*) instead of between useShortcutKeys and
tagsLoader—adjust the import order in RunFilters.tsx so useShortcutKeys and
tagsLoader remain together and ShortcutKey is grouped with components.
In `@apps/webapp/app/components/runs/v3/ScheduleFilters.tsx`:
- Around line 75-85: Define a single named constant for the sentinel select
value (e.g., export const UNSET_VALUE = '__unset__') and replace all raw "ALL"
string literals in ScheduleFilters (including inside handleChange, other change
handlers, defaults, and option value attributes) with this constant so
comparisons (selected === UNSET_VALUE), default values, and option value props
stay in sync; update URL handling logic that sets/deletes the "type" param to
treat UNSET_VALUE as meaning unset and ensure any tests or consumers reference
the new constant as well.
In `@apps/webapp/app/components/runs/v3/SharedFilters.tsx`:
- Around line 1035-1044: The apply function is unnecessarily wrapped in
useCallback and captures a stale closure (missing clearSearchValue in deps);
remove the useCallback wrapper and replace it with a plain inline function (or
const apply = () => { ... } defined without useCallback) used at the JSX click
site, ensuring it still calls clearSearchValue(), replace({ cursor: undefined,
direction: undefined, [paramKey]: inputValue === "" ? undefined :
inputValue?.toString() }), and setOpen(false); this eliminates the incorrect
dependency array and stale-closure risk around clearSearchValue, inputValue,
replace, paramKey, and setOpen.
In `@apps/webapp/app/hooks/useFuzzyFilter.ts`:
- Around line 36-61: The returned setter setFilterText currently only updates
internalFilterText and is a no-op when controlledFilterText is provided; update
useFuzzyFilter's API docs and behavior to avoid misleading callers: in the JSDoc
for useFuzzyFilter (and the exported return shape) explicitly state that
setFilterText only updates internal state when uncontrolled (i.e., when
controlledFilterText is undefined) and will not override a provided
controlledFilterText, and optionally replace the raw setInternalFilterText
return with a conditional setter that either calls setInternalFilterText or,
when controlledFilterText is present, logs a clear warning/returns a no-op;
reference the symbols filterText, setFilterText, internalFilterText,
controlledFilterText, and function useFuzzyFilter so reviewers can locate and
apply the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9454039f-f83a-4d5c-8fbb-14476a32d9cc
📒 Files selected for processing (45)
apps/webapp/app/components/MachineLabelCombo.tsxapps/webapp/app/components/logs/LogsLevelFilter.tsxapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/components/logs/LogsTaskFilter.tsxapps/webapp/app/components/logs/LogsVersionFilter.tsxapps/webapp/app/components/metrics/ModelsFilter.tsxapps/webapp/app/components/metrics/OperationsFilter.tsxapps/webapp/app/components/metrics/PromptsFilter.tsxapps/webapp/app/components/metrics/ProvidersFilter.tsxapps/webapp/app/components/metrics/QueuesFilter.tsxapps/webapp/app/components/metrics/ScopeFilter.tsxapps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/components/primitives/Buttons.tsxapps/webapp/app/components/primitives/Input.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/components/query/QueryEditor.tsxapps/webapp/app/components/runs/v3/AIFilterInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/runs/v3/ScheduleFilters.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/runs/v3/TaskRunsTable.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsxapps/webapp/app/hooks/useFuzzyFilter.tsapps/webapp/app/presenters/v3/BuiltInDashboards.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.batches/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardKey/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.models._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHistoryPopover.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules.new/route.tsxapps/webapp/app/services/runsRepository/runsRepository.server.ts
💤 Files with no reviewable changes (1)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx
| const hasAppliedFilters = | ||
| tasks.length > 0 || | ||
| queues.length > 0 || | ||
| models.length > 0 || | ||
| prompts.length > 0 || | ||
| operations.length > 0 || | ||
| providers.length > 0; |
There was a problem hiding this comment.
Count scope and time changes as applied filters.
hasAppliedFilters ignores scope, period, from, and to, so the new “Clear all filters” button never appears when a user changes only the dashboard scope or time range. Because this lives in MetricDashboard, the same gap affects both built-in and custom dashboards.
Possible fix
const hasAppliedFilters =
tasks.length > 0 ||
queues.length > 0 ||
models.length > 0 ||
prompts.length > 0 ||
operations.length > 0 ||
- providers.length > 0;
+ providers.length > 0 ||
+ scope !== "environment" ||
+ from !== undefined ||
+ to !== undefined ||
+ (period !== undefined && period !== defaultPeriod);Also applies to: 306-316
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardKey/route.tsx
around lines 248 - 254, hasAppliedFilters currently only checks
tasks/queues/models/prompts/operations/providers and ignores time and scope
changes; update the logic in MetricDashboard where hasAppliedFilters is defined
(the const hasAppliedFilters) to also include checks for scope, period, from,
and to (e.g., scope !== defaultScope OR period !== defaultPeriod OR from/to
timestamps present or different from defaults) so that changing dashboard scope
or time range counts as an applied filter; make the same change to the other
occurrence of hasAppliedFilters in this file to ensure the “Clear all filters”
button appears when only scope/time range changes.
There was a problem hiding this comment.
Intentional. The "Clear all filters" button is scoped to the chip-style applied filters (tasks/queues/models/etc.). Scope and time are persistent controls that always render their current value via their own selectors — there's no chip to clear, and resetting them via this button would be a hidden side-effect users wouldn't expect. Happy to revisit if we get UX feedback that users want a single "reset everything" affordance, but as-is the current behavior matches the rest of the app (RunFilters, BatchFilters, WaitpointTokenFilters all only count chip filters in their pre-existing hasFilters check too — until this PR).
Side note: I did expand WaitpointTokenFilters.hasFilters to include period/from/to per a separate review comment. There's a real inconsistency between routes here that we should align on in a follow-up.
There was a problem hiding this comment.
@samejr, that's a clear rationale — scope and time selectors are persistent affordances with visible state of their own, so excluding them from the "Clear all filters" action avoids unexpected side-effects. My comment was off-base here.
Thanks for the note on the WaitpointTokenFilters inconsistency too — makes sense to track that alignment as a follow-up.
✏️ Learnings added
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3451
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardKey/route.tsx:248-254
Timestamp: 2026-05-03T15:29:39.933Z
Learning: In triggerdotdev/trigger.dev, the `hasAppliedFilters` / `hasFilters` flag that controls visibility of a "Clear all filters" button is intentionally scoped only to chip-style applied filters (e.g., tasks, queues, models, prompts, operations, providers). Persistent controls like `scope` and time range (`period`, `from`, `to`) have their own selectors that always render their current value, so they are deliberately excluded from this flag to avoid unexpected hidden side-effects when clearing. Do not flag this pattern as a bug or suggest adding scope/time checks to `hasAppliedFilters` in MetricDashboard or similar route components (e.g., `apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardKey/route.tsx`).
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3319
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions/route.tsx:179-189
Timestamp: 2026-04-02T19:18:26.255Z
Learning: In this repo’s route components that render the Inspector `ResizablePanelGroup` panels, it’s acceptable to pass `collapsed={!isShowingInspector}` together with a no-op `onCollapseChange={() => {}}` when panel visibility is intentionally controlled only by route parameters (e.g., `*Param` search/route params) rather than user drag/collapse interactions. Do not flag an empty/no-op `onCollapseChange` as “missing wiring” in these cases; only flag it when collapse state is expected to change based on user interaction.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/components/runs/v3/BatchFilters.tsx (1)
69-88:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInclude the time params in
hasFilters.
TimeFilteris now always visible, but this guard still only checksstatusesandid. When a user applies onlyperiod/from/to, the page has an active filter but the “Clear all filters” button never renders.Suggested fix
- const hasFilters = searchParams.has("statuses") || searchParams.has("id"); + const hasFilters = + searchParams.has("statuses") || + searchParams.has("id") || + searchParams.has("period") || + searchParams.has("from") || + searchParams.has("to");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/runs/v3/BatchFilters.tsx` around lines 69 - 88, The hasFilters boolean in the BatchFilters component only checks for "statuses" and "id", so the Clear all filters button is hidden when only time filters are set; update the hasFilters calculation in BatchFilters to also check for the time parameters used by TimeFilter (e.g., "period", "from", and "to") by testing searchParams.has("period") || searchParams.has("from") || searchParams.has("to") in addition to the existing checks so the conditional rendering of the Form/Button reflects active time filters.
🧹 Nitpick comments (1)
apps/webapp/app/components/primitives/SearchInput.tsx (1)
3-3: ⚡ Quick winDrop the
useCallbackwrappers around these local handlers.
handleSubmitandhandleClearare only used inside this component, so memoizing them adds dependency noise without providing useful stability here.♻️ Proposed simplification
-import { useCallback, useEffect, useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; @@ - const handleSubmit = useCallback(() => { + function handleSubmit() { const resetValues = Object.fromEntries(resetParams.map((p) => [p, undefined])); if (text.trim()) { replace({ [paramName]: text.trim(), ...resetValues }); } else { del([paramName, ...resetParams]); } - }, [text, replace, del, resetParams, paramName]); + } - const handleClear = useCallback(() => { + function handleClear() { setText(""); del([paramName, ...resetParams]); - }, [del, resetParams, paramName]); + }As per coding guidelines, "Only use
useCallback/useMemofor context provider values, expensive derived data that is a dependency elsewhere, or stable refs required by a dependency array".Also applies to: 41-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/primitives/SearchInput.tsx` at line 3, Remove the unnecessary useCallback wrappers around the local handlers in SearchInput.tsx: replace the memoized handleSubmit and handleClear (and any other handlers wrapped with useCallback around lines where they are defined) with plain function declarations inside the component, update their usages (e.g., onSubmit, onClick) to call the plain functions directly, and remove useCallback from the import list; keep the rest of the component logic unchanged so dependency arrays and props remain correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/webapp/app/components/runs/v3/BatchFilters.tsx`:
- Around line 69-88: The hasFilters boolean in the BatchFilters component only
checks for "statuses" and "id", so the Clear all filters button is hidden when
only time filters are set; update the hasFilters calculation in BatchFilters to
also check for the time parameters used by TimeFilter (e.g., "period", "from",
and "to") by testing searchParams.has("period") || searchParams.has("from") ||
searchParams.has("to") in addition to the existing checks so the conditional
rendering of the Form/Button reflects active time filters.
---
Nitpick comments:
In `@apps/webapp/app/components/primitives/SearchInput.tsx`:
- Line 3: Remove the unnecessary useCallback wrappers around the local handlers
in SearchInput.tsx: replace the memoized handleSubmit and handleClear (and any
other handlers wrapped with useCallback around lines where they are defined)
with plain function declarations inside the component, update their usages
(e.g., onSubmit, onClick) to call the plain functions directly, and remove
useCallback from the import list; keep the rest of the component logic unchanged
so dependency arrays and props remain correct.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d1c1a33f-2c47-4eab-aa35-5507a6630cb1
📒 Files selected for processing (11)
apps/webapp/app/components/metrics/ModelsFilter.tsxapps/webapp/app/components/metrics/ScopeFilter.tsxapps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/runs/v3/AIFilterInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsxapps/webapp/app/hooks/useFuzzyFilter.ts
✅ Files skipped from review due to trivial changes (1)
- apps/webapp/app/components/runs/v3/AIFilterInput.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/webapp/app/components/metrics/ModelsFilter.tsx
- apps/webapp/app/components/metrics/ScopeFilter.tsx
- apps/webapp/app/hooks/useFuzzyFilter.ts
- apps/webapp/app/components/runs/v3/RunFilters.tsx
- apps/webapp/app/components/runs/v3/SharedFilters.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathUse named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
apps/webapp/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Only use
useCallback/useMemofor context provider values, expensive derived data that is a dependency elsewhere, or stable refs required by a dependency array. Don't wrap ordinary event handlers or trivial computations
Files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
{apps,internal-packages}/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
pnpm run typecheckto verify changes in apps and internal packages (apps/*,internal-packages/*) instead ofbuild, which proves almost nothing about correctness
Files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
{package.json,**/*.{ts,tsx,js}}
📄 CodeRabbit inference engine (CLAUDE.md)
Pin Zod to version 3.25.76 exactly across the entire monorepo - never use a different version or version range
Files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from@trigger.dev/coreusing subpaths only, never the root export
Always import tasks from@trigger.dev/sdk, never from@trigger.dev/sdk/v3or deprecatedclient.defineJob
Add crumbs to code using//@Crumbscomments or `// `#region` `@crumbsblocks for debug tracing during development
Files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
**/*.{ts,tsx,js,jsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting is enforced using Prettier. Run
pnpm run formatbefore committing
Files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
🧠 Learnings (6)
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
📚 Learning: 2026-04-16T14:21:15.229Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/components/logs/LogsTaskFilter.tsx:135-163
Timestamp: 2026-04-16T14:21:15.229Z
Learning: When rendering lists of task registry items in apps/webapp (e.g., <SelectItem /> rows) and using `key={item.slug}`, do not flag it as potentially non-unique. In trigger.dev’s `TaskIdentifier` table, the DB constraint `@unique([runtimeEnvironmentId, slug])` guarantees `slug` is unique within a given runtime environment, so `item.slug` is safe as the React key as long as the list is derived from that registry/constraint (and not from a legacy query that could produce duplicate slugs).
Applied to files:
apps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
📚 Learning: 2026-03-22T13:32:43.005Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/metrics/OperationsFilter.tsx:85-110
Timestamp: 2026-03-22T13:32:43.005Z
Learning: In triggerdotdev/trigger.dev, the shared `FilterMenuProvider` (from `~/components/runs/v3/SharedFilters`) manages Ariakit `ComboboxProvider` state internally and exposes the current `search` value and `setSearch` via render props `(search, setSearch) => …`. For filter components (e.g., `OperationsFilter`, `ModelsFilter`, `QueuesFilter`, `ProvidersFilter`), treat the render-prop tuple and their use of `searchValue={search}` and `clearSearchValue={() => setSearch("")}` as the correct wiring. Do not raise a review finding about missing/broken connection between the ComboBox input and the search state—if the component follows this provider/render-prop pattern and the `filtered` `useMemo` dependencies are reactive, the state synchronization is expected to be handled by Ariakit through the provider.
Applied to files:
apps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
📚 Learning: 2026-03-22T13:32:44.229Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/metrics/ProvidersFilter.tsx:74-96
Timestamp: 2026-03-22T13:32:44.229Z
Learning: When reviewing components under `apps/webapp/app/components/runs/v3/`, avoid flagging “broken/unconnected search state” in filters that use `FilterMenuProvider` wrapping Ariakit’s `ComboboxProvider` and expose `(search, setSearch)` (render props). In this intentional pattern, the `searchValue` render-prop value should be treated as reactive (it re-renders on every keystroke), passed into the dropdown child, and used in `useMemo` to filter options. Do not require additional wiring beyond this established render-prop/ComboboxProvider integration.
Applied to files:
apps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx
🔇 Additional comments (3)
apps/webapp/app/components/navigation/DashboardDialogs.tsx (1)
118-149: LGTM!The
shortcutprop is correctly threaded fromCreateDashboardPageButtonintoButton, which handles keyboard registration internally viauseShortcutKeys. The type-only import ofShortcutDefinitionis clean, and the previously flagged unused imports have been removed. No issues found.apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx (1)
191-197: Remove theas anyref casts from these Ariakit triggers.These are the same type-safety escape hatches already called out in the sibling permanent-filter implementation. If the typed ref works in
BatchFilters.tsx, these four triggers should not needas anyeither.#!/bin/bash # Compare the Waitpoint trigger refs with the sibling BatchFilters implementation. # Expected result: the Waitpoint file still shows `ref={triggerRef as any}` in four places, # while BatchFilters shows the same pattern without the cast. sed -n '191,197p' apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx sed -n '356,362p' apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx sed -n '441,447p' apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx sed -n '525,531p' apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx sed -n '181,186p' apps/webapp/app/components/runs/v3/BatchFilters.tsx sed -n '272,276p' apps/webapp/app/components/runs/v3/BatchFilters.tsxAlso applies to: 356-362, 441-447, 525-531
apps/webapp/app/components/primitives/Select.tsx (1)
107-107:tooltipTitleprop wiring looks correct.The optional prop is cleanly added to both
SelectPropsandSelectTriggerProps, properly destructured, and forwarded throughSelect → SelectTrigger. All dependent callers (LogsRunIdFilter,ModelsFilter,OperationsFilter) supply bothtooltipTitleandshortcut, so the tooltip will render and display correctly.Also applies to: 131-131, 211-211, 269-270, 278-278
Lots of filter UX improvements across lots of routes
General
Route updates